remove documents without revs after user deletion

Author: escattone

kitsune/wiki/handlers.py

@@ -50,8 +50,21 @@ def on_user_deletion(self, user: User) -> None:
             based_on=None
         )
 
+        # Translations with one or more revisions whose based-on matches one of the
+        # revisions that will be deleted, will lose those revisions via cascade deletion.
+        # Gather these translations, so we can check later if they no longer have any
+        # revisions, and so need to be deleted themselves.
+        translations_affected = list(
+            Document.objects.filter(
+                parent__isnull=False, revisions__based_on__in=revs_to_delete
+            ).values_list("id", flat=True)
+        )
+
         Document.objects.filter(
             revisions__creator=user,
             current_revision__isnull=True,
         ).exclude(revisions__creator__in=User.objects.exclude(id=user.id)).delete()
         revs_to_delete.delete()
+
+        # Delete any translations that no longer have any revisions due to cascade deletions.
+        Document.objects.filter(revisions__isnull=True, id__in=translations_affected).delete()

kitsune/wiki/models.py

@@ -965,7 +965,9 @@ def latest_revision(excluded_rev, constraint):
             except IndexError:
                 return None
 
-        Revision.objects.filter(based_on=self).update(based_on=None)
+        # Block cascade deletions, via "based_on", of revisions of parent documents.
+        Revision.objects.filter(document__parent__isnull=True, based_on=self).update(based_on=None)
+
         document = self.document
 
         # If the current_revision is being deleted, try to update it to the

kitsune/wiki/tests/test_handlers.py

@@ -236,3 +236,46 @@ def test_revision_based_on_deletion(self):
         rev3 = Revision.objects.filter(id=rev3_id).first()
         self.assertIsNotNone(rev3, "The approved revision should not be deleted")
         self.assertEqual(rev3.based_on, rev2)
+
+    def test_revision_based_on_deletion_with_translation(self):
+        """
+        Test the handling of a revision's "based_on" field, when it references a revision
+        created by a user that is deleted, within the context of a translation.
+        """
+        doc = DocumentFactory()
+        rev1 = RevisionFactory(document=doc, creator=self.user)
+        rev2 = ApprovedRevisionFactory(document=doc, based_on=rev1)
+
+        de_doc = DocumentFactory(parent=doc, locale="de")
+        de_rev = RevisionFactory(document=de_doc, based_on=rev1)
+
+        it_doc = DocumentFactory(parent=doc, locale="it")
+        it_rev1 = RevisionFactory(document=it_doc, based_on=rev1)
+        it_rev2 = RevisionFactory(document=it_doc, based_on=rev2)
+
+        doc_id = doc.id
+        rev1_id = rev1.id
+        rev2_id = rev2.id
+        de_doc_id = de_doc.id
+        de_rev_id = de_rev.id
+        it_doc_id = it_doc.id
+        it_rev1_id = it_rev1.id
+        it_rev2_id = it_rev2.id
+
+        self.listener.on_user_deletion(self.user)
+
+        # Ensure that the based-on handling of revisions within parent-less
+        # documents is correct.
+        self.assertTrue(Document.objects.filter(id=doc_id).exists())
+        self.assertFalse(Revision.objects.filter(id=rev1_id).exists())
+        self.assertTrue(Revision.objects.filter(id=rev2_id, based_on=None).exists())
+
+        # Ensure that the un-approved translated revision is cascade deleted,
+        # and also that its document, if it no longer has any revisions, is
+        # also deleted.
+        self.assertFalse(Revision.objects.filter(id=de_rev_id).exists())
+        self.assertFalse(Document.objects.filter(id=de_doc_id).exists())
+
+        self.assertFalse(Revision.objects.filter(id=it_rev1_id).exists())
+        self.assertTrue(Revision.objects.filter(id=it_rev2_id, based_on=rev2).exists())
+        self.assertTrue(Document.objects.filter(id=it_doc_id).exists())